Emit missing-fields diagnostics from inference#22336
Conversation
rustbot
added
the
S-waiting-on-review
| remaining_fields.contains_key(&field.name).then_some(field_idx) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| if !missing_fields.is_empty() { |
There was a problem hiding this comment.
This if is redundant, we already checked that above.
| expr_store_diagnostics(db, acc, source_map); | ||
|
|
||
| let infer = InferenceResult::of(db, id); | ||
| let mut delayed_missing_fields_diagnostics = Vec::new(); |
There was a problem hiding this comment.
Why do you delay this? I see no reason for that.
This comment has been minimized.
This comment has been minimized.
|
@Twil3akine, what's the status of this? |
Co-authored-by: Chayim Refael Friedman <chayimfr@gmail.com>
Use the current type-owner variable when emitting delayed missing-fields diagnostics after the upstream diagnostics plumbing rename. AI assistance disclosure: ChatGPT helped identify and validate this minimal compile fix. I reviewed the change before committing.
Twil3akine
force-pushed
the
missing-fields-inference
branch
from
2042451 to
f0e1250
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Thanks for the follow-up. I've rebased the PR and resolved the conflicts. Regarding the diagnostic ordering change mentioned above: should I proceed with updating the tests so they don't depend on the diagnostic order? I'd appreciate your thoughts. |
|
Diagnostics order should not affect anything. If there are tests that depend on it, that's weird and maybe a bug. Feel free to update them. Definitely don't complicate the code to change ordering, anyway. |
|
Sorry for the late reply. This may be due to my implementation, but removing the delay causes some tests to fail. Would it be okay to proceed by removing the delay and updating the tests accordingly? |
|
Remove it, update the tests, and I'll see if there is a bug in them. |
Remove the delayed emission of missing-fields inference diagnostics and make diagnostic fix tests select the matching assist result instead of depending on diagnostic ordering. AI assistance disclosure: I used ChatGPT to help implement and verify this follow-up change.
|
I done, pushed the update. I removed the delayed emission and updated the diagnostic fix test helper so it selects the matching fix result instead of relying on diagnostic ordering. Could you take another look when you have time? |
3 participants
Implements one diagnostic FIXME from #22140.
This changes record-literal missing-fields diagnostics to be emitted from inference, while reusing the existing
MissingFieldsdiagnostic.MissingFieldsalready represents record literal/pattern missing field diagnostics and is rendered as rustc hard errorE0063, so adding a separate diagnostic would duplicate behavior.This PR:
MissingFieldsdiagnosticValidated with:
AI assistance disclosure:
I used ChatGPT to help review the implementation plan and draft this PR description. I reviewed the generated suggestions before submitting.